fix(api): map backend errors to 400/404 instead of 500 (EN-1227) by flemzord · Pull Request #190 · formancehq/flows

flemzord · 2026-06-11T08:50:04Z

Problem (H7 + H8 — HIGH)

H8: most read/write handlers mapped every backend error to 500. Unknown ids (readInstance, readWorkflow, runWorkflow, postEvent, abortWorkflowInstance, readInstanceHistory) returned 500 instead of 404, contradicting the spec and creating alerting noise.
H7: createTrigger / testTrigger returned 500 for a malformed JSON body, and createWorkflow returned 500 for config-validation failures (and paniced on a post-wait GetInstance and a json.Marshal).

Fix

New api.WriteError(w, r, err) central mapper: sql.ErrNoRows + workflow not-found sentinels + Temporal NotFound ⇒ 404; workflow.ErrInvalidConfig ⇒ 400; otherwise 500. Used across the read/run/post/abort/history/test handlers (v1 + v2).
createTrigger / testTrigger: malformed body ⇒ 400.
createWorkflow: Create now wraps validation errors as workflow.ErrInvalidConfig (⇒ 400); both panics replaced with proper error responses.

Tests

TestGetInstanceNotFound (404) and TestCreateWorkflowValidationError (400).

Severity: HIGH.

Note: this is the v1/v2-duplicated layer (see L13 in the review). Edits internal/workflow/manager.go (Create + a new sentinel) and many handler files shared with other PRs in this series — independent, different regions; merges may need trivial resolution.

Most read/write handlers mapped every backend error to 500, and the trigger/workflow create+test handlers returned 500 for malformed request bodies. Unknown ids therefore looked like server faults and bad client payloads were misclassified. - Add api.WriteError(), mapping sql.ErrNoRows, the workflow not-found sentinels and Temporal NotFound to 404, ErrInvalidConfig to 400, and everything else to 500. Use it in readInstance, readWorkflow, runWorkflow, postEvent, abortWorkflowInstance, readInstanceHistory and testTrigger (v1 + v2). - createTrigger / testTrigger: malformed body -> 400 (was 500). - createWorkflow: wrap validation failures as workflow.ErrInvalidConfig so they surface as 400; drop two panics (post-wait GetInstance, json.Marshal) in favour of returned error responses. Tests: TestGetInstanceNotFound (404) and TestCreateWorkflowValidationError (400). Note: edits internal/workflow/manager.go (Create + new sentinel) and many v1/v2 handlers shared with other PRs in this series; independent, different regions.

coderabbitai · 2026-06-11T08:50:49Z

Walkthrough

This PR refactors API error handling to intelligently classify backend errors and map them to appropriate HTTP responses. A new WriteError helper function replaces generic 500-error responses, distinguishing not-found errors (404), configuration validation errors (400), and server errors (500). Workflow manager configuration validation failures are wrapped with ErrInvalidConfig sentinel. All v1 and v2 handler error paths are updated to use the new helper, and several handlers are improved to treat JSON decode failures as client validation errors and eliminate panic paths.

Changes

API Error Handling Infrastructure and Refactoring

Layer / File(s)	Summary
Error classification and mapping foundation `internal/api/errors.go`, `internal/workflow/manager.go`	New `WriteError` function routes errors to 404/400/500 responses based on type. `ErrInvalidConfig` sentinel marks config validation failures as client errors. `WorkflowManager.Create` wraps validation errors with the sentinel.
API v1 and v2 handlers refactored to use WriteError `internal/api/v1/handler_.go`, `internal/api/v2/handler_.go`	All handlers (read, create, post, abort, run endpoints in both versions) replace generic error responses with intelligent `WriteError` classification. JSON decode errors are treated as validation failures (400). Panic paths are replaced with error responses.
Test coverage for error classification behavior `internal/api/v2/handler_create_workflow_test.go`, `internal/api/v2/handler_read_instance_test.go`	New tests verify validation errors return 400 and not-found errors return 404, confirming end-to-end error classification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through error logs,
Classifying faults in code's fog—
404, 400, 500 too,
Each mapped right, each response true. ✨
No more panics in the night,
Just WriteError setting things right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description comprehensively covers the changes across all modified files, clearly explaining the problems (H7/H8), the fixes implemented, and the test additions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: mapping backend errors to appropriate HTTP status codes (400/404) instead of defaulting to 500, directly supported by the file summaries showing error-handling refactors across multiple handlers.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/api-error-mapping

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

internal/api/errors.go (1)

13-31: ⚡ Quick win

Add direct branch-level tests for the error mapper.

This helper now defines core API status semantics. Please add table-driven unit tests that exercise each branch (sql.ErrNoRows, workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound, Temporal NotFound, workflow.ErrInvalidConfig, and default 500) to prevent silent regressions across handlers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/errors.go` around lines 13 - 31, Add table-driven unit tests for
WriteError that exercise each branch: provide inputs of sql.ErrNoRows,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound, a Temporal NotFound
(serviceerror.NotFound{}), workflow.ErrInvalidConfig, and a generic error for
the default case. For each case call WriteError with an
httptest.ResponseRecorder and a dummy *http.Request and assert the resulting
HTTP status and/or body matches the expected handler (sharedapi.NotFound -> 404,
sharedapi.BadRequest -> 400 with "VALIDATION" code,
sharedapi.InternalServerError -> 500). Name the test function
TestWriteError_TableDriven and place cases in a slice with descriptive names so
any failure shows which branch regressed; use the same identifiers WriteError,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound,
workflow.ErrInvalidConfig, sql.ErrNoRows, and serviceerror.NotFound to locate
the logic under test.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/api/errors.go`:
- Around line 13-31: Add table-driven unit tests for WriteError that exercise
each branch: provide inputs of sql.ErrNoRows, workflow.ErrInstanceNotFound,
workflow.ErrWorkflowNotFound, a Temporal NotFound (serviceerror.NotFound{}),
workflow.ErrInvalidConfig, and a generic error for the default case. For each
case call WriteError with an httptest.ResponseRecorder and a dummy *http.Request
and assert the resulting HTTP status and/or body matches the expected handler
(sharedapi.NotFound -> 404, sharedapi.BadRequest -> 400 with "VALIDATION" code,
sharedapi.InternalServerError -> 500). Name the test function
TestWriteError_TableDriven and place cases in a slice with descriptive names so
any failure shows which branch regressed; use the same identifiers WriteError,
workflow.ErrInstanceNotFound, workflow.ErrWorkflowNotFound,
workflow.ErrInvalidConfig, sql.ErrNoRows, and serviceerror.NotFound to locate
the logic under test.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18477be9-0d2f-46e7-93a8-fa7089f405d8

📥 Commits

Reviewing files that changed from the base of the PR and between 271bf8d and 8b00f5a.

📒 Files selected for processing (21)

internal/api/errors.go
internal/api/v1/handler_abort_workflow_instance.go
internal/api/v1/handler_create_trigger.go
internal/api/v1/handler_create_workflow.go
internal/api/v1/handler_post_event.go
internal/api/v1/handler_read_instance.go
internal/api/v1/handler_read_instance_history.go
internal/api/v1/handler_read_workflow.go
internal/api/v1/handler_run_workflow.go
internal/api/v2/handler_abort_workflow_instance.go
internal/api/v2/handler_create_trigger.go
internal/api/v2/handler_create_workflow.go
internal/api/v2/handler_create_workflow_test.go
internal/api/v2/handler_post_event.go
internal/api/v2/handler_read_instance.go
internal/api/v2/handler_read_instance_history.go
internal/api/v2/handler_read_instance_test.go
internal/api/v2/handler_read_workflow.go
internal/api/v2/handler_run_workflow.go
internal/api/v2/handler_test_trigger.go
internal/workflow/manager.go

flemzord · 2026-06-11T09:30:47Z

Tracked in Jira: EN-1227 (Epic EN-1217).

flemzord · 2026-08-05T11:46:47Z

Superseded by #199, which consolidates this change with the related reliability and safety fixes on top of the current main branch.

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

flemzord changed the title ~~fix(api): map backend errors to 400/404 instead of 500~~ fix(api): map backend errors to 400/404 instead of 500 (EN-1227) Jun 11, 2026

flemzord mentioned this pull request Aug 5, 2026

fix: consolidate reliability and safety fixes #199

Open

flemzord closed this Aug 5, 2026

Navigation Menu

Uh oh!

fix(api): map backend errors to 400/404 instead of 500 (EN-1227) - #190

fix(api): map backend errors to 400/404 instead of 500 (EN-1227)#190
flemzord wants to merge 1 commit into
mainfrom
fix/api-error-mapping

flemzord commented Jun 11, 2026

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

flemzord commented Jun 11, 2026 •

edited by atlassian Bot

Loading

Uh oh!

flemzord commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

flemzord commented Jun 11, 2026

Problem (H7 + H8 — HIGH)

Fix

Tests

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

flemzord commented Jun 11, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flemzord commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

flemzord commented Jun 11, 2026 •

edited by atlassian Bot

Loading